Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add client options testing #562

Merged
merged 1 commit into from
May 11, 2022
Merged

Conversation

nsmith5
Copy link
Contributor

@nsmith5 nsmith5 commented May 9, 2022

Summary

Adds some tests for the WithTimeout and WithUserAgent options for the client. While this legacy client will be deprecated soon, these were some easy tests to write so why not? Not sure if we're keeping library API compat in the deprecation to GRPC, but if so perhaps these can be good tests to make sure we reimplement this when cutting over to the new client.

Ticket Link

Release Note

NONE

Signed-off-by: Nathan Smith <nathan@chainguard.dev>
@codecov-commenter
Copy link

Codecov Report

Merging #562 (2be7cb1) into main (969e796) will increase coverage by 2.35%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #562      +/-   ##
==========================================
+ Coverage   36.67%   39.03%   +2.35%     
==========================================
  Files          19       19              
  Lines        1445     1445              
==========================================
+ Hits          530      564      +34     
+ Misses        855      819      -36     
- Partials       60       62       +2     
Impacted Files Coverage Δ
pkg/ca/fileca/watch.go 50.00% <0.00%> (-50.00%) ⬇️
pkg/api/client.go 35.77% <0.00%> (+35.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 969e796...2be7cb1. Read the comment docs.

Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gRPC generates its own clients, so I think the plan will be to just remove the API client entirely.

@nsmith5
Copy link
Contributor Author

nsmith5 commented May 9, 2022

Okie, We can drop this if GRPC cut over is happening soon. I was just looking around the repo for easy test coverage to add honestly 😆

@@ -0,0 +1,62 @@
//
// Copyright 2021 The Sigstore Authors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Copyright 2021 The Sigstore Authors.
// Copyright 2022 The Sigstore Authors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right we should update the boilerplate everywhere and the boilerplate test to match...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, this is added when you create the file first time.

if is a new file you add the current year

@dlorenc dlorenc merged commit ce5565f into sigstore:main May 11, 2022
@nsmith5 nsmith5 deleted the test-client-options branch May 11, 2022 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants